Preserve SHOTGUN_API_CACERTS precedence in shotgun_api3 cert patch#1122
Merged
stevelittlefish merged 2 commits intoJul 20, 2026
Merged
Conversation
_patch_shotgun_api3_certs() unconditionally returned the Core-extracted cacert.pem whenever ca_certs wasn't explicitly passed, silently ignoring the documented SHOTGUN_API_CACERTS environment variable. When the extracted cert lives on a slow network share, this made every cold ShotGrid connection pay a large one-time filesystem cost (~20-30s observed in Nuke 17) that a local cert copy avoids. Restore shotgun_api3's documented precedence: explicit ca_certs -> SHOTGUN_API_CACERTS -> extracted Core cert fallback. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
carlos-villavicencio-adsk
approved these changes
Jul 17, 2026
carlos-villavicencio-adsk
left a comment
carlos-villavicencio-adsk
left a comment
Contributor
There was a problem hiding this comment.
LGTM. Just one comment to be considered.
Contributor
There was a problem hiding this comment.
Pull request overview
This pull request restores shotgun_api3’s documented CA cert precedence in tank_vendor._patch_shotgun_api3_certs() so that SHOTGUN_API_CACERTS is honored when no explicit ca_certs argument is provided, avoiding unnecessary slow reads of the Core-extracted cert in some environments.
Changes:
- Update the
shotgun_api3cert patch to respectSHOTGUN_API_CACERTSwhenca_certsis not explicitly provided. - Add a new test suite validating precedence across: explicit
ca_certsarg, env var (including empty string), and extracted cert fallback.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| tests/core_tests/test_tank_vendor.py | Adds a dedicated test class to validate cert precedence behavior for the shotgun_api3 cert patch. |
| python/tank_vendor/init.py | Adjusts _patched_get_certs_file() to preserve SHOTGUN_API_CACERTS precedence before falling back to the extracted cert. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
setUp() captured shotgun_api3.Shotgun._get_certs_file via attribute access, which unwraps the staticmethod descriptor to a plain function. Restoring that plain function in tearDown re-assigned it as a regular instance method, so any Shotgun() constructed by a later test in the same process passed both self and ca_certs into a function that only accepts ca_certs, breaking unrelated tests (e.g. util_tests/test_shotgun_connect.py) with: TypeError: _patched_get_certs_file() takes from 0 to 1 positional arguments but 2 were given Snapshot the raw descriptor from Shotgun.__dict__ instead, so the staticmethod wrapper is preserved on restore. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
![hound[bot]](https://avatars.githubusercontent.com/in/3598?s=60&v=4){kind=small}
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #1122 +/- ##
=======================================
Coverage 80.10% 80.10%
=======================================
Files 203 203
Lines 19529 19529
=======================================
Hits 15643 15643
Misses 3886 3886
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
stevelittlefish
deleted the
ticket/SG-44256-shotgun-api-cacerts-precedence
branch
July 20, 2026 10:18
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
_patch_shotgun_api3_certs()intank_vendor/__init__.pycurrently returns the Core-extractedcacert.pemunconditionally wheneverca_certsisn't explicitly passed, silently bypassing the documentedSHOTGUN_API_CACERTSenvironment variable.shotgun_api3's own documented precedence: explicitca_certsargument →SHOTGUN_API_CACERTSenv var → extracted Core cert fallback.Test plan
TestShotgunAPI3CertPatchPrecedenceintests/core_tests/test_tank_vendor.py, calling_patch_shotgun_api3_certs()directly against a throwaway fakepkgs.zip/certs/layout so behavior can be verified without depending on the real build-time layout:SHOTGUN_API_CACERTSset → wins over the extracted cert (the regression this fixes)ca_certsargument → still outranks everything, including the env varcore_testssuite (510 tests) passes with the fix applied, no regressions.Follow-up fix (review feedback)
CI/review caught that the new test's
tearDownwas leaking global state across test modules when the full suite ran in one process:Root cause:
setUpcapturedshotgun_api3.Shotgun._get_certs_filevia plain attribute access, which unwraps the installedstaticmethoddescriptor down to a raw function.tearDownthen reassigned that raw function directly as the class attribute, turning_get_certs_fileback into a regular instance method. AnyShotgun(...)constructed by a later test in the same process (e.g.tests/util_tests/test_shotgun_connect.py) then calledself._get_certs_file(ca_certs), which passed bothselfandca_certsinto a function that only acceptsca_certs.Fix: snapshot the raw descriptor via
shotgun_api3.Shotgun.__dict__["_get_certs_file"]instead of attribute access, so thestaticmethodwrapper is preserved on restore.core_tests.test_tank_vendor+util_tests.test_shotgun_connectin the same process (matches how the full suite runs).